Use many connections on the benchmarks server#7852
Conversation
| } | ||
|
|
||
| fn connection(&self) -> Result<Connection> { | ||
| let root = self.root.lock(); |
There was a problem hiding this comment.
i don't love that this still requires locking, even though the criticial section is pretty short lived...guess it's fine 🤷
There was a problem hiding this comment.
I think that we cant really get around this? Also it is unlikely that this would have contention anyways
There was a problem hiding this comment.
this method is called on every request right?
There was a problem hiding this comment.
at this point, why do we even need the lock? can't we just clone here?
There was a problem hiding this comment.
the type isn't send so it needs to be wrapped in something with interior mut
There was a problem hiding this comment.
Oh I guess it might have some contention, but duckdb is going to have a lot of overhead in creating txns for all of these requests (which is further dominated by the actual queries) so I don't think it's a problem
connortsui20
force-pushed
the
ct/many-connections
branch
from
3ad0f97 to
c00f12f
Compare
Merging this PR will not alter performance
|
connortsui20
force-pushed
the
ct/many-connections
branch
3 times, most recently
from
03f8731 to
5e07956
Compare
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
connortsui20
force-pushed
the
ct/many-connections
branch
from
c35640d to
9625ab4
Compare
3 participants
Summary
Turns out we can just clone the connection and have each of them write (with retry). Now all threads can read and write concurrently instead of being serialized through a lock.
Testing
What's that?!